Skip to content

feat(scope): Emit buffer_overflow discard reason when popping breadcrumbs #3993

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

feat(scope): Emit buffer_overflow discard reason when popping breadcrumbs #3993

wants to merge 1 commit into from

Conversation

@AbhiPrasad
Copy link
Member

@AbhiPrasad AbhiPrasad commented Jan 24, 2025

ref getsentry/team-sdks#116

This PR implements a new client discard reason for buffer_overflow. This will be used to track when the internal breadcrumbs buffer overflows for the new logs product that we are working on.

log_item is the new data category for logs, which we are basing out of breadcrumbs. Eventually we will be sending standalone envelope items that only contain breadcrumbs.

@AbhiPrasad AbhiPrasad self-assigned this Jan 24, 2025
@AbhiPrasad AbhiPrasad mentioned this pull request Jan 24, 2025
Closed
@codecov
Copy link

codecov bot commented Jan 24, 2025 •
edited
Loading

❌ 3 Tests Failed:

Tests completed Failed Passed Skipped
22080 3 22077 5708
View the top 3 failed tests by shortest run time 
tests.integrations.aiohttp.test_aiohttp test_crumb_capture
Stack Traces | 0.002s run time 
file .../integrations/aiohttp/test_aiohttp.py, line 476
  @pytest.mark.asyncio
  async def test_crumb_capture(
      sentry_init, aiohttp_raw_server, aiohttp_client, loop, capture_events
  ):
      def before_breadcrumb(crumb, hint):
E       fixture 'loop' not found
>       available fixtures: DictionaryContaining, ObjectDescribedBy, StringContaining, _capture_internal_warnings, _session_event_loop, aiohttp_client, aiohttp_client_cls, aiohttp_raw_server, aiohttp_server, benchmark, cache, capfd, capfdbinary, caplog, capsys, capsysbinary, capture_envelopes, capture_events, capture_events_forksafe, capture_exceptions, capture_record_lost_event_calls, clean_scopes, cov, doctest_namespace, event_loop, event_loop_policy, httpserver, httpsserver, internal_exceptions, maybe_monkeypatched_threading, monkeypatch, no_cover, pytestconfig, record_property, record_testsuite_property, record_xml_attribute, recwarn, render_span_tree, reset_integrations, sentry_init, smtpserver, suppress_deprecation_warnings, teardown_profiling, .../integrations/aiohttp/test_aiohttp.py::<event_loop>, tests/integrations/aiohttp::<event_loop>, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory, uninstall_integration, unused_tcp_port, unused_tcp_port_factory, unused_udp_port, unused_udp_port_factory, validate_event_schema
>       use 'pytest --fixtures [testpath]' for help on them.

.../integrations/aiohttp/test_aiohttp.py:476
tests.integrations.aiohttp.test_aiohttp test_crumb_capture
Stack Traces | 0.002s run time 
file .../integrations/aiohttp/test_aiohttp.py, line 476
  @pytest.mark.asyncio
  async def test_crumb_capture(
      sentry_init, aiohttp_raw_server, aiohttp_client, loop, capture_events
  ):
      def before_breadcrumb(crumb, hint):
E       fixture 'loop' not found
>       available fixtures: DictionaryContaining, ObjectDescribedBy, StringContaining, _capture_internal_warnings, _session_event_loop, aiohttp_client, aiohttp_client_cls, aiohttp_raw_server, aiohttp_server, benchmark, cache, capfd, capfdbinary, caplog, capsys, capsysbinary, capture_envelopes, capture_events, capture_events_forksafe, capture_exceptions, capture_record_lost_event_calls, clean_scopes, cov, doctest_namespace, event_loop, event_loop_policy, httpserver, httpsserver, internal_exceptions, maybe_monkeypatched_threading, monkeypatch, no_cover, pytestconfig, record_property, record_testsuite_property, record_xml_attribute, recwarn, render_span_tree, reset_integrations, sentry_init, smtpserver, suppress_deprecation_warnings, teardown_profiling, .../integrations/aiohttp/test_aiohttp.py::<event_loop>, tests/integrations/aiohttp::<event_loop>, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory, uninstall_integration, unused_tcp_port, unused_tcp_port_factory, unused_udp_port, unused_udp_port_factory, validate_event_schema
>       use 'pytest --fixtures [testpath]' for help on them.

.../integrations/aiohttp/test_aiohttp.py:476
tests.integrations.stdlib.test_httplib test_http_timeout
Stack Traces | 0.263s run time 
.../integrations/stdlib/test_httplib.py:373: in test_http_timeout
    assert len(transaction["spans"]) == 1
E   assert 0 == 1
E     +0
E     -1

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@AbhiPrasad AbhiPrasad force-pushed the abhi-python-sdk-breadcrumbs-buffer-overflow branch from 51e600f to 2621c08 Compare January 24, 2025 17:59
@sentrivana
Copy link
Contributor

sentrivana commented Jan 30, 2025

@AbhiPrasad The change looks good to me but we should have a test for this.

I imagine we can come up with a test case by combining some of the client report tests in https://github.com/getsentry/sentry-python/blob/master/tests/test_transport.py and this max_breadcrumbs test.

Do you have some bandwidth to look into adding a test case? Otherwise I can try coming up with something.

@AbhiPrasad AbhiPrasad closed this Feb 19, 2025
@AbhiPrasad AbhiPrasad deleted the abhi-python-sdk-breadcrumbs-buffer-overflow branch February 19, 2025 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@antonpirker antonpirker Awaiting requested review from antonpirker

@sentrivana sentrivana Awaiting requested review from sentrivana

Assignees

@AbhiPrasad AbhiPrasad

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AbhiPrasad @sentrivana